feat(engine)!: add audio latency reporting and tail compensation - #1998
feat(engine)!: add audio latency reporting and tail compensation#1998yuto-trd wants to merge 24 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesAudio Latency Query and Tail Recovery API
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a report-only audio latency API to the Beutl audio graph/effects layer so hosts and pipelines can query per-node/effect latency (notably Limiter lookahead) as a prerequisite for future compensation.
Changes:
- Introduces
GetLatencySamples(int sampleRate)onAudioNodeandAudioEffect(defaulting to0) and an aggregateAudioNode.GetTotalLatencySamples(...)that folds upstream latency vialocal + max(inputs). - Implements limiter lookahead latency reporting via
LimiterParameters.ToLatencySamples(...)and overrides onLimiterNode/LimiterEffect; addsAudioEffectGroupsummation behavior mirroringCreateNode. - Adds NUnit coverage for limiter latency behavior, effect/group aggregation semantics, and non-positive sample rate guards.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Beutl.UnitTests/Engine/Audio/AudioLatencyTests.cs | New fixture validating limiter latency reporting, aggregation rules, and argument validation. |
| src/Beutl.Engine/Audio/Graph/AudioNode.cs | Adds node latency reporting APIs and upstream aggregation method. |
| src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs | Overrides node latency reporting to expose lookahead delay. |
| src/Beutl.Engine/Audio/Effects/AudioEffect.cs | Adds effect-level latency reporting API for pre-graph queries. |
| src/Beutl.Engine/Audio/Effects/AudioEffectGroup.cs | Implements group latency as sum of enabled children (serial cascade). |
| src/Beutl.Engine/Audio/Effects/LimiterEffect.cs | Overrides effect latency reporting to match limiter lookahead (when enabled). |
| src/Beutl.Engine/Audio/Effects/LimiterParameters.cs | Adds shared helper to compute/clamp lookahead latency in samples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Validate sampleRate in GetTotalLatencySamples itself rather than relying on the indirect throw via GetLatencySamples, so an override that folds the upstream recursion before delegating still honors the contract. Cover the aggregating entry point in the non-positive-rate test. Addresses Copilot review feedback on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Beutl.Engine/Audio/Graph/AudioNode.cs`:
- Around line 46-50: In the Flush method of the AudioNode class, modify the
logic to throw an exception when there are multiple inputs (_inputs.Count > 1)
instead of silently returning a silent buffer. Keep the existing fallback
behavior that returns silence when there are zero inputs, but add a throw
statement for the multi-input case to force explicit merge or drain semantics in
subclasses that override Flush with multiple inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f7b50aeb-087e-4a90-971d-93276c749796
📒 Files selected for processing (4)
src/Beutl.Engine/Audio/Graph/AudioNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cstests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7f1bda716
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs`:
- Around line 74-78: The drained buffer is leaking when ProcessTail throws an
exception during the flush operation. In the AudioNode.Flush method, wrap the
ProcessTail call in a try-finally block to ensure the drained buffer is properly
disposed regardless of whether ProcessTail succeeds or throws an exception. The
finally block should dispose of the drained buffer after ProcessTail completes,
whether successfully or via exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a860065-c4a2-4d25-b6e2-f7d65fbaa530
📒 Files selected for processing (10)
src/Beutl.Engine/Audio/Graph/AudioNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/CompressorNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/EqualizerNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/GainNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/MixerNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/SourceNode.cstests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs
✅ Files skipped from review due to trivial changes (1)
- src/Beutl.Engine/Audio/Graph/Nodes/EqualizerNode.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c0f588207
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The single-input Flush path disposed the drained buffer only after a successful ProcessTail, leaking it on a throw. Wrap ProcessTail in try/catch and dispose the drain on failure; Dispose is idempotent, so a transforming node that already consumed the buffer is unaffected. Addresses CodeRabbit review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
The single-input Flush path disposed the drained buffer only after a successful ProcessTail, leaking it on a throw. Wrap ProcessTail in try/catch and dispose the drain on failure; Dispose is idempotent, so a transforming node that already consumed the buffer is unaffected. Addresses CodeRabbit review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
ac81c39 to
b5c71df
Compare
LimiterNode reports the animated worst-case lookahead, but the effect-level GetLatencySamples a host queries before graph construction still returned the CurrentValue snapshot, so it could under-reserve and reintroduce the tail loss. Mirror the node: an animated Lookahead reports MaxLookaheadMs. Addresses Codex review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 583146a1e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs (2)
153-182: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis test passes trivially and does not exercise
AppendFlushedTail.The window
Context(TimeSpan.Zero, clipSamples)covers the clip exactly, sonewBuffer.SampleCount == clipSamples,writeOffset == clipSamples, andcapacity == 0—AppendFlushedTailreturns early without draining (the samecapacity == 0situation the nested-clip test explicitly documents at Lines 288-289). The asserted indices[clipSamples - L, clipSamples)fall inside the main delayed-limiter slice, which is real non-zero sine regardless of recovery, so the assertion holds even if tail recovery is a complete no-op. To actually validate the recovery path, extend the window past the clip end so trailing capacity exists and assert on[clipSamples, clipSamples + L).💚 Proposed fix to exercise the drain path
- // A window that exactly covers the whole clip: it reaches the clip's true end, so ClipNode - // drains the limiter tail into the trailing samples. - using var output = clip.Process(Context(TimeSpan.Zero, clipSamples)); - - var data = output.GetChannelData(0); - bool tailNonZero = false; - for (int i = clipSamples - L; i < clipSamples; i++) + // A window that extends L samples past the clip end leaves trailing room, so AppendFlushedTail + // drains the limiter's held tail into samples [clipSamples, clipSamples + L). + using var output = clip.Process(Context(TimeSpan.Zero, clipSamples + L)); + + var data = output.GetChannelData(0); + bool tailNonZero = false; + for (int i = clipSamples; i < clipSamples + L; i++) { if (MathF.Abs(data[i]) > 1e-5f) { tailNonZero = true; break; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs` around lines 153 - 182, The test ClipNode_TerminalWindow_AppendsRecoveredTail does not actually exercise the AppendFlushedTail recovery path because the Context window exactly covers the clip with no trailing capacity, causing AppendFlushedTail to return early. To fix this, extend the Context window past the clip end to create trailing capacity (so the window sample count exceeds clipSamples), and then update the assertion loop indices to check the newly recovered tail samples in the range beyond the original clip end (approximately clipSamples to clipSamples + L) instead of checking indices within the clip that already contain valid sine data from the delayed limiter output.
185-208: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
ClipNode_ZeroLookahead_IsNoOpis tautological — it can never fail.
clipA/clipBuse identical configurations and the identicalContext(TimeSpan.Zero, clipSamples), andRangeSineNodeis deterministic, soa[i] == b[i]always holds regardless of behavior. The comment claims the reference is "A mid-clip window (does not reach the end)," butclipBuses the same terminal window asclipA. Either make the reference genuinely bypass the drain (e.g., a window that stops short of the clip end, comparing the overlapping region) or compare against the raw source output so the assertion has discriminating power.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs` around lines 185 - 208, The test ClipNode_ZeroLookahead_IsNoOp is tautological because clipA and clipB use identical configurations and both process with Context(TimeSpan.Zero, clipSamples), so a[i] will always equal b[i] regardless of the drain behavior. Fix this by modifying the reference (clipB) to use a different window that genuinely represents a mid-clip region that does not reach the end, such as Context(TimeSpan.Zero, clipSamples - someOffset), so the overlapping region comparison can actually detect if the drain perturbs samples, or alternatively compare against the raw sourceB output directly instead of a processed clipB output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs`:
- Around line 153-182: The test ClipNode_TerminalWindow_AppendsRecoveredTail
does not actually exercise the AppendFlushedTail recovery path because the
Context window exactly covers the clip with no trailing capacity, causing
AppendFlushedTail to return early. To fix this, extend the Context window past
the clip end to create trailing capacity (so the window sample count exceeds
clipSamples), and then update the assertion loop indices to check the newly
recovered tail samples in the range beyond the original clip end (approximately
clipSamples to clipSamples + L) instead of checking indices within the clip that
already contain valid sine data from the delayed limiter output.
- Around line 185-208: The test ClipNode_ZeroLookahead_IsNoOp is tautological
because clipA and clipB use identical configurations and both process with
Context(TimeSpan.Zero, clipSamples), so a[i] will always equal b[i] regardless
of the drain behavior. Fix this by modifying the reference (clipB) to use a
different window that genuinely represents a mid-clip region that does not reach
the end, such as Context(TimeSpan.Zero, clipSamples - someOffset), so the
overlapping region comparison can actually detect if the drain perturbs samples,
or alternatively compare against the raw sourceB output directly instead of a
processed clipB output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d48db3cc-c99a-4a33-a44d-1f11c1033a3a
📒 Files selected for processing (5)
src/Beutl.Engine/Audio/Graph/AudioNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/ClipNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cssrc/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cstests/Beutl.UnitTests/Engine/Audio/AudioLatencyCompensationTests.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Beutl.Engine/Audio/Graph/Nodes/DelayNode.cs
- src/Beutl.Engine/Audio/Graph/Nodes/LimiterNode.cs
- src/Beutl.Engine/Audio/Graph/AudioNode.cs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ff93c5e26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f054212294
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef513eb5f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c772e5949c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fac660636a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Lookahead-bearing audio effects (the Limiter's lookahead delay line) make the rendered clip lag and drop its tail by `lookaheadSamples`, but the audio graph had no way to report that latency. This adds a report-only API so the pipeline (and plugin hosts) can discover per-node and per-effect latency; it is the prerequisite for any future automatic compensation. - AudioNode.GetLatencySamples(sampleRate) (virtual, default 0) reports a node's own latency; GetTotalLatencySamples aggregates the feeding path as local + max(input totals), so a linear cascade sums and a mixer fan-in takes the slowest branch (both virtual/overridable for plugin authors). - AudioEffect.GetLatencySamples(sampleRate) lets a host query latency before a graph node exists; AudioEffectGroup sums its enabled children. - LimiterNode/LimiterEffect override it via a new LimiterParameters.ToLatencySamples helper that mirrors the runtime clamp. Report-only and behavior-preserving: the new methods are side-effect-free and are not called from Process/Compose, so render output is unchanged. Actual compensation (range/flush) is deferred — the board's range-expansion proposal is incompatible with the stateful chunked pull graph (it would trigger spurious LimiterNode resets and double-process overlap at every chunk boundary), so it needs a dedicated render-session layer. Refs: Project #9 "AI Review" item "Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects" Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
Validate sampleRate in GetTotalLatencySamples itself rather than relying on the indirect throw via GetLatencySamples, so an override that folds the upstream recursion before delegating still honors the contract. Cover the aggregating entry point in the non-positive-rate test. Addresses Copilot review feedback on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
Recovers the clip-tail audio that a lookahead effect (the Limiter) leaves stuck in its delay line. Adds AudioNode.Flush(context): a node drains the latency it still holds, treating the upstream source as exhausted (a leaf returns silence), so a trimmed clip never bleeds real audio. The default passes a single input's drain through unchanged; LimiterNode overrides it to push its lookahead delay line out through the same path Process uses. ClipNode, on the window that reaches the clip's true end, calls Inputs[0].Flush over the clip-local range [Duration, Duration+L) — contiguous with the main slice, so the cached effect state never resets — and appends the drained tail into the trailing pad the window already reserves. Crucially this does NOT expand the Composer's requested range: doing so pushes consecutive preview windows outside LimiterNode's 1-tick contiguity tolerance and triggers a spurious Reset + double-processed overlap at every chunk boundary (verified wrong in design review). Compensation is a node contract instead, so exact window tiling and cached DSP state are preserved. Scope: tail recovery only. Leading lookahead silence is left intact (removing it would shift the clip and break A/V sync — documented-intended behavior). Refs: Project #9 "AI Review" item "Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects" Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
Addresses the CodeRabbit/Codex review of the Flush compensation: the default Flush bypassed the node's own processing, hardcoded stereo, dropped fan-in branches, and under-reported animated lookahead. Unify everything on a ProcessTail(input, context) seam: Process feeds it real upstream audio, Flush feeds it the drained tail, so a transforming node shapes the tail exactly like the body. The base ProcessTail is pass-through, so the zero-processing path stays byte-identical. - AudioNode: default Flush now runs the upstream drain through ProcessTail (so a downstream Equalizer/Compressor/Gain processes the recovered tail); zero-input silence matches the last processed channel count instead of hardcoded stereo; a fan-in node without a Flush override now throws rather than silently dropping tails. - Limiter/Gain/Equalizer/Compressor/Delay: Process bodies move into ProcessTail (LimiterNode's bespoke Flush is gone — it is now just a ProcessTail override). - MixerNode: overrides Flush to drain and mix every branch, recovering a lookahead tail held in any fan-in branch. - LimiterNode.GetLatencySamples: reports the worst-case lookahead when animated, so the drain reserves enough room when automation peaks near the clip end. - SourceNode records its stereo output so the flush silence matches. ClipNode's chunk-boundary-alignment tail loss (block ending exactly at the clip end) is documented as a known limitation; the only fixes are out of scope (range expansion resets the limiter; an oversized terminal buffer breaks the output contract). Refs: Project #9 "AI Review" item "Audio graph: latency reporting/compensation API for lookahead-bearing AudioEffects" Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
The single-input Flush path disposed the drained buffer only after a successful ProcessTail, leaking it on a throw. Wrap ProcessTail in try/catch and dispose the drain on failure; Dispose is idempotent, so a transforming node that already consumed the buffer is unaffected. Addresses CodeRabbit review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
LimiterNode reports the animated worst-case lookahead, but the effect-level GetLatencySamples a host queries before graph construction still returned the CurrentValue snapshot, so it could under-reserve and reintroduce the tail loss. Mirror the node: an animated Lookahead reports MaxLookaheadMs. Addresses Codex review on PR #1998. Claude-Session: https://claude.ai/code/session_01PJ9eDL52jpiM1t5oDRCLM2
A SoundGroup mixes child clips and then flushes them through the group's own clip in the group's time domain. A nested ClipNode inherited the base single-input Flush, which forwarded the parent's drain context unchanged, so the child's cached lookahead limiter saw a discontinuity, reset its delay line, and dropped the very tail being drained. Override ClipNode.Flush to rebuild the drain at clip-local Duration — where the clip's last Process slice ended — mirroring AppendFlushedTail. The parent's start is intentionally dropped, which is also why an intervening ShiftNode needs no flush override of its own. Document the remaining best-effort gaps (animated-lookahead drain, custom-leaf channel count, DelayNode latency budget) inline as known limitations / follow-ups rather than expanding the PR further.
ClipNode.Flush drained from the clip's natural Duration, which is wrong when a parent (a SoundGroup window) trims the clip before its own end: the cached effects last processed up to the trim boundary, so draining from Duration skips past them, trips the limiter's discontinuity guard, and drops the tail held at the boundary. Track the clip-local end of the last processed window and drain from there instead. Document the remaining best-effort gaps inline as known limitations: MixerNode.Flush drains every branch unconditionally (a child that ended early on the chunk-alignment edge can emit a stale tail late), and SceneNode reports zero latency for a referenced scene (an inner limiter tail is dropped at a SceneSound cut).
A disabled AudioEffectGroup contributes no nodes to the graph (Sound.Compose skips a disabled effect's CreateNode), yet GetLatencySamples still summed its enabled children, so a pre-graph latency query disagreed with the built graph. Return 0 when the group itself is disabled, matching LimiterEffect. Also harden the surrounding latency-reporting surface from PR review: - LimiterNode.GetLatencySamples guards sampleRate at the override boundary so a future early-return cannot bypass the contract. - AudioEffect.GetLatencySamples documents that an override must agree with its node's GetLatencySamples and return 0 when IsEnabled is false. - Add Flush-path tests for DelayNode/CompressorNode/EqualizerNode covering the Process->ProcessTail buffer-ownership transfer on the drain.
…de flush gap Adds two Composer-level tests closing the coverage gap on AppendEndedSoundTails' suppression branches: a non-contiguous (seek) window must not inject the previous clip's stale limiter tail (IsContiguous early return), and InvalidateCache must drop the recorded previous window so a subsequent contiguous window flushes nothing. Both mirror Composer_FlushesSoundEndingExactlyAtTheWindowBoundary and assert silence where that test asserts a recovered tail. Also documents SpeedNode's missing Flush override (same class of limitation as ResampleNode's, already documented there): a latency-bearing node placed upstream of a SpeedNode would drop its tail on flush. Comment-only; no behavior change.
fac6606 to
c029568
Compare
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; current eligibility rejects muted, disabled, or removed sounds, while dirty-state and captured-range checks prevent edited or moved sounds from flushing stale cached tails. Important Files Changed
Reviews (10): Last reviewed commit: "fix(review): harden audio cache and inpu..." | Re-trigger Greptile |
Code Review BotNo comment/code divergences or documentation drift detected. Reviewed 69 file(s); skipped 1. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c029568521
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61ae3ef3d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BREAKING CHANGE: CompositionFrame now requires a CompositionEligibility constructor argument. Frame producers must capture target-eligible EngineObject identities independently of time-range intersection and pass CompositionEligibility.Empty when no objects are eligible.
|
Addressed the boundary-aligned disappearance case in 05b8d84. CompositionFrame now carries a required, reference-identity-safe CompositionEligibility snapshot; SceneCompositor captures target eligibility independently of time-range intersection while respecting element/object enablement and layer mute/solo policy. Composer flushes a natural-end tail only when that sound remains eligible in the next frame. Added regressions for natural endings, exact-boundary ineligibility, out-of-range eligibility, mute/disable filtering, and identity semantics. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05b8d843af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfeca8e16d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
BREAKING CHANGE: In Beutl.Engine, CompositionFrame.Eligibility is now nullable: null means no snapshot was captured, while CompositionEligibility.Empty means a captured empty set. Audio composition requires a non-null eligibility snapshot. Multi-input AudioNode subclasses must override Flush with their own merge semantics; ProcessTail owns its input and must dispose it before returning a different buffer or throwing. Beutl.ProjectSystem graphics producers should pass null instead of building an unused eligibility snapshot.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06a023e223
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 215e7ae62e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e64d0a790b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f5635ba4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Minimum allowed line rate is |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e755eb3e14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
No TODO comments were found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c1e504027
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!IsSameTimestamp(entry.SoundRange.End, previous.End)) | ||
| continue; |
There was a problem hiding this comment.
Continue partially drained top-level tails
When a sound ends inside the previous render block with less trailing capacity than its reported latency—for example, a 5 ms limiter ending 2 ms before the block boundary—ClipNode appends only the available 2 ms and retains the remaining drain state, but this exact-end check rejects the entry in the next block because SoundRange.End != previous.End. The remaining 3 ms is therefore never flushed; recently ended entries with outstanding latency need to remain eligible for draining rather than limiting recovery to sounds ending exactly at the boundary.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
Description
Implements Project #9 board item #77 end to end: host-queryable audio latency reporting plus tail-loss compensation for latency-bearing audio effects.
Reporting
AudioNode.GetLatencySamples(int)and path-awareGetTotalLatencySamples(int)reporting. Linear cascades sum latency, fan-in takes the slowest branch, and unbounded or overflowing budgets saturate atint.MaxValue.AudioEffect.GetLatencySamples(int)so hosts can query latency before a graph node exists.LimiterEffectandLimiterNodereport lookahead delay; enabled children inAudioEffectGroupcontribute serially.Tail recovery
AudioNode.Flush(AudioProcessContext)drain contract so latency held after the last normalProcessblock can be emitted without reading real post-clip source audio.SpeedNodedrains through the source timeline, scales source-domain latency budgets into output-domain samples, handles animated easing overshoot conservatively, and keeps 44.1 kHz source reads sample-exact without timestamp drift.Composition eligibility
Composition now captures target eligibility separately from current time-range intersection. This lets an ended latency-bearing object participate in a contiguous tail window while preventing stale tails after seeks, cache invalidation, or object moves.
Affected areas
Beutl.EngineBeutl.UnitTestsBreaking changes
BREAKING CHANGE: The
Beutl.Enginecomposition and audio extensibility contracts changed.CompositionFramenow requires a fourthCompositionEligibilityconstructor argument, and its generated deconstruction shape includes that value. Multi-inputAudioNodesubclasses must overrideFlush(AudioProcessContext)with their own merge semantics; the base implementation throws because it cannot infer a correct fan-in policy.ProcessTailowns its input and must dispose it before returning a different buffer or propagating an exception.Migration:
EngineObjectidentities independently of time-range intersection and pass the snapshot to the constructor. UseCompositionEligibility.Emptywhen eligibility was captured and no objects are eligible;nullis reserved for frames where eligibility was not captured.Flushby draining and combining every upstream branch according to the node's normal fan-in semantics.MixerNode.Flushis the built-in reference implementation.ProcessTailoverrides must dispose the received input when returning a different buffer or throwing; returning the same buffer transfers it back to the caller.Verification
dotnet format Beutl.slnx --no-restoredotnet build Beutl.slnx --no-restore: 0 errors (existing dependency/nullability warnings remain)References